Skip to content

Implement declarative (macro_rules!) derive macros (RFC 3698) #145208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Implement declarative (macro_rules!) derive macros (RFC 3698) #145208

wants to merge 2 commits into from
+561 −29

Conversation

joshtriplett
Copy link
Member

This is a draft for review, and should not be merged yet.

This is layered atop #145153 , and has
only two additional commits atop that. The first handles parsing and provides a
test for various parse errors. The second implements expansion and handles
application.

This implements RFC 3698, "Declarative (macro_rules!) derive macros".
Tracking issue: #143549

This has one remaining issue, which I could use some help debugging: in
tests/ui/macros/macro-rules-derive-error.rs, the diagnostics for
derive(fn_only) (for a fn_only with no derive rules) and
derive(ForwardReferencedDerive) both get emitted twice, as a duplicate
diagnostic.

From what I can tell via adding some debugging code,
unresolved_macro_suggestions is getting called twice from
finalize_macro_resolutions for each of them, because
self.single_segment_macro_resolutions has two entries for the macro, with two
different parent_scope values. I'm not clear on why that happened; it doesn't
happen with the equivalent code using attrs.

I'd welcome any suggestions for fixing this.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Aug 10, 2025
@joshtriplett
Copy link
Member Author

r? @petrochenkov

@joshtriplett joshtriplett removed T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-clippy Relevant to the Clippy team. labels Aug 10, 2025
@joshtriplett
Copy link
Member Author

(Removing T-clippy and T-rustdoc, because the two commits specific to this PR don't touch them.)

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@joshtriplett joshtriplett added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2025
@joshtriplett
Copy link
Member Author

(Marking as "waiting on review" because I'm seeking help with the test failure here.)

@bors

This comment was marked as resolved.

Sign in to view
@petrochenkov
Copy link
Contributor

Blocked on #145153.
@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 11, 2025
@rustbot rustbot added the T-clippy Relevant to the Clippy team. label Aug 11, 2025
@joshtriplett joshtriplett removed the T-clippy Relevant to the Clippy team. label Aug 11, 2025
@rust-log-analyzer

This comment was marked as outdated.

Sign in to view
@rustbot rustbot added the T-clippy Relevant to the Clippy team. label Aug 12, 2025
@joshtriplett joshtriplett removed the T-clippy Relevant to the Clippy team. label Aug 12, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@joshtriplett joshtriplett added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 12, 2025
@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 12, 2025 •
edited
Loading

(Marking for review now that 145153 is queued.)

@petrochenkov
Copy link
Contributor

(I'll wait until the rebase anyway.)

@rustbot rustbot added the T-clippy Relevant to the Clippy team. label Aug 12, 2025
@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 12, 2025 •
edited
Loading

@petrochenkov Rebased atop the latest version of 145153. (All but the last two commits will go away once 145153 is merged.)

@joshtriplett joshtriplett removed the T-clippy Relevant to the Clippy team. label Aug 12, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@petrochenkov
Copy link
Contributor

I meant after #145153 is merged, the github interface makes reviewing stacked PRs quite inconvenient, so I basically never look at them.

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2025
@joshtriplett
Copy link
Member Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 14, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@joshtriplett
mbe: Parse macro derive rules
5094e9c 
This handles various kinds of errors, but does not allow applying the
derive yet.

This adds the feature gate `macro_derive`.
@joshtriplett
mbe: Handle applying macro_rules derives
a9f6032 
Add infrastructure to apply a derive macro to arguments, consuming and
returning a `TokenTree` only.

Handle `SyntaxExtensionKind::MacroRules` when expanding a derive, if the
macro's kinds support derive.

Add tests covering various cases of `macro_rules` derives.
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu-llvm-19-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
28 
+ error: cannot find derive macro `fn_only` in this scope
+   --> $DIR/macro-rules-derive-error.rs:30:14
+    |
+ LL | macro_rules! fn_only {
+    |              ------- `fn_only` exists, but has no `derive` rules
+ ...
+ LL |     #[derive(fn_only)]
+    |              ^^^^^^^
+    |
+    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+ 
29 error: cannot find macro `DeriveOnly` in this scope
30   --> $DIR/macro-rules-derive-error.rs:33:5
31    |

---
+   --> $DIR/macro-rules-derive-error.rs:42:14
+    |
+ LL | macro_rules! ForwardReferencedDerive {
+    |              ^^^^^^^^^^^^^^^^^^^^^^^
+    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+ 
+ error: aborting due to 7 previous errors
51 
52 

Note: some mismatched output was normalized before being compared
-   --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:30:14
-   --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:38:10
-   --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:42:14
+ error: cannot find derive macro `fn_only` in this scope
+   --> $DIR/macro-rules-derive-error.rs:30:14
+    |
+ LL | macro_rules! fn_only {
+    |              ------- `fn_only` exists, but has no `derive` rules
+ ...
+ LL |     #[derive(fn_only)]
+    |              ^^^^^^^
+    |
+    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+ 
+ error: cannot find derive macro `ForwardReferencedDerive` in this scope
+   --> $DIR/macro-rules-derive-error.rs:38:10
+    |
+ LL | #[derive(ForwardReferencedDerive)]
+    |          ^^^^^^^^^^^^^^^^^^^^^^^ consider moving the definition of `ForwardReferencedDerive` before this call
+    |
+ note: a macro with the same name exists, but it appears later
+   --> $DIR/macro-rules-derive-error.rs:42:14
+    |
+ LL | macro_rules! ForwardReferencedDerive {
+    |              ^^^^^^^^^^^^^^^^^^^^^^^
+    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+ 
+ error: aborting due to 7 previous errors


The actual stderr differed from the expected stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args macros/macro-rules-derive-error.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/macros/macro-rules-derive-error.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/macros/macro-rules-derive-error" "-A" "unused" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"
stdout: none
--- stderr -------------------------------
error: MyDerive: struct S1;
##[error]  --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:5:9
   |
LL |         compile_error!(concat!("MyDerive: ", stringify!($($body)*)));
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL |     #[derive(MyDerive)]
   |              -------- in this derive macro expansion
   |
   = note: this error originates in the derive macro `MyDerive` (in Nightly builds, run with -Z macro-backtrace for more info)

error: cannot find macro `MyDerive` in this scope
##[error]  --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:27:5
   |
LL |     MyDerive!(arg);
   |     ^^^^^^^^
   |
   = note: `MyDerive` is in scope, but it is a derive macro: `#[derive(MyDerive)]`

error: cannot find derive macro `fn_only` in this scope
##[error]  --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:30:14
   |
LL | macro_rules! fn_only {
   |              ------- `fn_only` exists, but has no `derive` rules
...
LL |     #[derive(fn_only)]
   |              ^^^^^^^

error: cannot find derive macro `fn_only` in this scope
##[error]  --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:30:14
   |
LL | macro_rules! fn_only {
   |              ------- `fn_only` exists, but has no `derive` rules
...
LL |     #[derive(fn_only)]
   |              ^^^^^^^
   |
   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: cannot find macro `DeriveOnly` in this scope
##[error]  --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:33:5
   |
LL | macro_rules! DeriveOnly {
   |              ---------- `DeriveOnly` exists, but has no rules for function-like invocation
...
LL |     DeriveOnly!(); //~ ERROR: cannot find macro `DeriveOnly` in this scope
   |     ^^^^^^^^^^

error: cannot find derive macro `ForwardReferencedDerive` in this scope
##[error]  --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:38:10
   |
---
  --> /checkout/tests/ui/macros/macro-rules-derive-error.rs:42:14
   |
LL | macro_rules! ForwardReferencedDerive {
   |              ^^^^^^^^^^^^^^^^^^^^^^^
   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 7 previous errors
------------------------------------------

petrochenkov
petrochenkov reviewed Aug 14, 2025
View reviewed changes
compiler/rustc_expand/messages.ftl
@@ -70,7 +70,7 @@ expand_invalid_fragment_specifier =
invalid fragment specifier `{$fragment}`
.help = {$help}
expand_macro_args_bad_delim = macro attribute argument matchers require parentheses
expand_macro_args_bad_delim = macro `{$rule_kw}` argument matchers require parentheses
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expand_macro_args_bad_delim = macro `{$rule_kw}` argument matchers require parentheses
expand_macro_args_bad_delim = `{$rule_kw}` macro argument matchers require parentheses

Nit: the current wording reads weirdly.

@petrochenkov
Copy link
Contributor

From what I can tell via adding some debugging code,
unresolved_macro_suggestions is getting called twice from
finalize_macro_resolutions for each of them, because
self.single_segment_macro_resolutions has two entries for the macro, with two
different parent_scope values. I'm not clear on why that happened; it doesn't
happen with the equivalent code using attrs.

Probably because of this FIXME in expand.rs

                                    // FIXME: Consider using the derive resolutions (`_exts`)
                                    // instead of enqueuing the derives to be resolved again later.

I don't think it's too important to fix, identically looking diagnostics are not shown to users anyway (UI tests suite specifically disables that deduplication).

It's suspicious that the parent_scope values are different though.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@joshtriplett @rust-log-analyzer @bors @petrochenkov @rustbot